I am trying to use NiceMenus to allow for a horizontal drop box primary menu, but use DHTML_Menu for the sidebar menu. When I do this the dhtml_menu expand functionality stops and no arrow image on the links appear at the highest menu level. If I click a link with children then on that page the arrow image appears and the menu with expand/contract as normal.
I have turned off dhtml_menu for my primary menu and can see the no_dhtml part in the original primary menus, but when I use the nice menus primary menu block it does not have the no_dhtml part. Is this the problem?
If this is not easy to fix, does anyone have another suggested option to have a horizontal drop box primary menu?
Thanks.
Comments
Comment #1
cburschkaI understand part of the problem, but not quite all of it.
If you disable DHTML Menu entirely, does the primary menu then do what you want?
If so, then the patch I wrote for #352005: Ubercart DHTML conflict - use module-specific selector might fix this one. It makes DHTML Menu use a special CSS class that ensures it only adds its Javascript functionality to the menus that it actually controls.
This way, you should be able to use NiceMenus for your primary horizontal menu, and DHTML Menu for your sidebar menu, without causing a conflict.
Comment #2
kiz_0987 commentedThanks. Actually the Nice Menus part seems to work fine (it is CSS only I think unless using IE, which I am not). It is the dhtml_menu that stops functioning (as described). I was wondering if the fact that the nice menu block does not have the 'no_dhtml' class may disrupt the dhtml_menu for its own menus.
Comment #3
kiz_0987 commentedTested with 6.x-3.4 in the hope that the Ubercart fix would help me. Still does not work (as described above). Is there any more info I can provide to help this issue? Thanks.
Comment #4
kiz_0987 commentedI looked into this a bit. It is not a JS issue (confirmed by creating a working static html copy without nice_menus and then manually adding the nice_menus part -- both dhtml_menu and nice_menus will work fine). The problem is in the menu being generated by dhtml_menu. When nice_menus is present all the expanded menus are missing from the html source (all rendered as leaf). In dhtml_menu_theme_menu_item all the $has_children flags are 0 and so the expanded menu never gets rendered. Next step is to figure out why $has_children is incorrectly 0 when nice_menus is present. Any ideas?
Comment #5
kiz_0987 commentedI continued to look at this and can get nice_menus to work with a kludge. The problem seems to be that when nice_menus calls theme('menu_item_link') (which it does in 2 places) this is intercepted by dhtml_menu. This should be OK and in fact the primary-links menu (the nice_menu) is rendered correctly, but the *other* dhtml_menus are not. Maybe it's an issue with the dhtml_menu stack? Anyway if I change nice_menus to not call the theme function but a local version of the theme_menu_item_link then all will work OK (not that this is really a solution, just a kludge).
This problem occurs whether primary-links is disabled or not in the dhtml_menu preferences.
Comment #6
traviscarden commentedSubscribing
Comment #7
erikwebb commentedSubscribe.
Comment #8
ferrangil commentedSubscribing.
Looking forward to have Ubercart Catalog working with dhtml menu...
Comment #9
erikwebb commentedI solved this problem the same as post #5. I redirected Nice Menu's call to theme('menu_item_link') to a local function. I just copied the theme_menu_item_link() code from the API and everything seems to be working now. I've attached the patch file.
Comment #10
dboulet commentedI think that the problem is that this module wrongfully assumes that there will be one call to
theme('menu_item')for each call totheme('menu_item_link'). As mentioned in #5, this module intercepts calls to these theme functions and adds the link to the dhtml stack—the problem is that items are only removed from the stack on thetheme('menu_item')call, a step skipped my the Nice menus module.My fix for this is to send only the current stack item to the
_dhtml_menu_subtree()function, instead of the entire stack, since it can contain extra unneeded items.Comment #11
cburschkaAnd this works? The entire reason for the stack is because the menu has to have a path to get the sub-tree, since it may have to load a sub-tree at any depth. I'll make sure to test this with items that are in the active trail or always expanded.
Comment #12
cburschkaAs expected, this breaks all menus below the active trail. Visit /admin, and all its sub-items appear as leaves, because the "admin" item was already expanded by the core menu.inc (active trail is expanded), and DHTML Menu had to load the items below /admin/build etc. To find /admin/build in the tree, it needs to know where /admin is, requiring the entire stack.
Comment #13
dboulet commentedYou're right Arancaytar, big -1 for #10. I had noticed that
_dhtml_menu_subtree()always returned an empty array if there was a Nice menu on the page, and I thought that the problem was with the stack, but maybe it's something else.Comment #14
cburschkaWell, the problem *is* with the stack, naturally. It's just that getting rid of the stack isn't as trivial as a one-liner - it has to be replaced with a better approach.
I have experimented with expanding the tree-indexer already contained in DHTML Menu (for translating mlid into the sortable "weight-mlid-name" key used by the tree) to also index parents. If each item's parent path is contained in the index, then the module only needs the final item in the stack to find it - and any previous calls to theme_menu_item_link by nicemenu are just ignored.
Here's a patch for that. Haven't tested with NiceMenu yet, but it seems to work on its own - good start.
The extra indexing may cost some performance, but not noticably so.
Comment #15
dboulet commentedPatch in #14 works fine for me, thanks Arancaytar.
Comment #16
cburschkaBased on that, I have committed this to DRUPAL-6--3. I'm rushing it a bit, but it's an annoying bug and I could find no problems in the self-review.
This patch requires porting to D7. It is already implemented in the massive patch in #473356: DHTML Menu takes another leap, but needs to be isolated and committed separately. I've found that rewriting the same change three times is a passable substitute for a second pair of eyes.
Comment #17
cburschkaAfter this fix, 6.x-3.5 will likely be released soon, but I would appreciate it very much if someone could test the 6.x-3.x-dev snapshot once it updates... getting that tested will greatly speed up the release process.
Comment #18
executex commentedThank you very much... 3.5 fixed it perfectly.
Keep up the excellent work, you're fast!!
Comment #19
cburschkaYep =)
Okay, here comes a patch for HEAD.
Comment #20
cburschkaStill applies, and I didn't notice any problems when I tested it two months ago. This being D7, I just commit it and see if it works as it should.
Done.
Comment #21
cburschkaAugh.
Step 1:
- Upload a patch late at night and tell yourself you're going to test it later
- Two months down the road, commit the patch on the basis that "I don't remember anything breaking"
- Introduce syntax errors.
- Headdesk.
Also, I neglected a logic change required in the _stack() function, where only the popped element must be returned, rather than the whole stack.
Comment #22
cburschkaHere's an incremental fix for both problems. It's pretty vital to get this in as my module's HEAD is broken since yesterday. >_<
Comment #23
cburschkaNow in HEAD.